Remove to_bytes / from_bytes from non-top-level Python wrappers#845
Merged
Conversation
Drop bytes serialization from component-level Python wrappers (`Function`, `Linear`, `Quadratic`, `Polynomial`, `Parameter`, `NamedFunction`, `EvaluatedNamedFunction`, `SampledNamedFunction`, `DecisionVariable`, `EvaluatedDecisionVariable`, `SampledDecisionVariable`). Bytes serialization remains on the top-level container types (`Instance`, `ParametricInstance`, `Solution`, `SampleSet`) and the cross-evaluate DTOs (`State`, `Samples`, `Parameters`). The component bytes round-trip is lossy with respect to metadata that lives on the parent container, so exposing it from individual elements invites silent data loss. Drop the API surface ahead of the metadata-storage refactor that makes the lossiness explicit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR narrows the Python (PyO3) binding surface by removing to_bytes / from_bytes from non-top-level “component” wrapper types, keeping bytes serialization only on top-level container types and cross-evaluate DTOs to avoid metadata-lossy round-trips.
Changes:
- Removed
to_bytes/from_bytesmethods from component-level wrappers (e.g.,DecisionVariable,Function,Linear,NamedFunction, evaluated/sampled variants). - Regenerated Python stubs (
__init__.pyi) and API reference JSON to reflect the updated public surface. - Removed Python tests that exercised now-deleted component-level bytes round-trips.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| python/ommx/src/decision_variable.rs | Removes bytes serialization APIs from DecisionVariable wrapper. |
| python/ommx/src/evaluated_decision_variable.rs | Removes bytes serialization APIs from EvaluatedDecisionVariable wrapper. |
| python/ommx/src/sampled_decision_variable.rs | Removes bytes serialization APIs from SampledDecisionVariable wrapper. |
| python/ommx/src/function.rs | Removes bytes serialization APIs from Function wrapper. |
| python/ommx/src/linear.rs | Removes bytes serialization APIs from Linear wrapper. |
| python/ommx/src/quadratic.rs | Removes bytes serialization APIs from Quadratic wrapper. |
| python/ommx/src/polynomial.rs | Removes bytes serialization APIs from Polynomial wrapper. |
| python/ommx/src/parameter.rs | Removes bytes serialization APIs from Parameter wrapper. |
| python/ommx/src/named_function.rs | Removes bytes serialization APIs from NamedFunction wrapper. |
| python/ommx/src/evaluated_named_function.rs | Removes bytes serialization APIs from EvaluatedNamedFunction wrapper. |
| python/ommx/src/sampled_named_function.rs | Removes bytes serialization APIs from SampledNamedFunction wrapper. |
| python/ommx/ommx/_ommx_rust/init.pyi | Updates generated stubs to remove the deleted methods from affected classes. |
| python/ommx-tests/tests/test_named_function.py | Removes NamedFunction bytes round-trip test that no longer applies. |
| docs/api/api_reference.json | Updates generated API reference to remove the deleted methods from affected classes. |
…ytes Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
termoshtt
added a commit
that referenced
this pull request
Apr 28, 2026
Same treatment as 1.x — the 2.x release notes pin code samples to API shapes that no longer compile under v3 (e.g. property-style `*_df` accessors, the bytes-on-element-types methods removed in #845). Build the pages but don't try to execute them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
termoshtt
added a commit
that referenced
this pull request
Apr 30, 2026
## Summary - Move `name` / `subscripts` / `parameters` / `description` off `NamedFunction`, `EvaluatedNamedFunction`, `SampledNamedFunction` onto a new `NamedFunctionMetadataStore` SoA store, kept as a sibling field on `Instance` / `ParametricInstance` / `Solution` / `SampleSet`. Same snapshot-model PyO3 wrappers and `WithMetadata`-based pandas rendering as PR #843 (which did this for `Constraint` / `DecisionVariable`). - Parse boundary moves to per-collection: `Vec<v1::NamedFunction>::Parse` returns `(BTreeMap, NamedFunctionMetadataStore)`; per-element helpers (`named_function_to_v1`, `evaluated_named_function_to_v1`, `sampled_named_function_to_v1`) reattach metadata at emit. - `Instance::evaluate` / `evaluate_samples` thread the host store onto the produced `Solution` / `SampleSet`. `SampleSet::get` carries the store onto each per-sample `Solution`. - Element-level `to_bytes` / `from_bytes` removed from `NamedFunction` / `EvaluatedNamedFunction` / `SampledNamedFunction` — same rationale as #845: a per-element wire round-trip can no longer carry host-stored metadata, so the methods became misleading rather than safe. - METADATA_STORAGE_V3.md flipped \`NamedFunction\` from \"deferred — separate PR\" to \"landed (this PR)\". ## End state After this PR the v3 SoA metadata story is uniform across all four metadata-bearing element types (decision variables, regular / indicator / one-hot / sos1 constraints, named functions). The only remaining deferred item in METADATA_STORAGE_V3.md is the two-mode Standalone / Attached wrapper design for write-through metadata mutation. ## Test plan - [x] \`cargo test -p ommx --lib\` — 475 passed (incl. new \`test_instance_roundtrip_preserves_named_function_metadata\` regression test) - [x] \`task python:test\` — all adapter + ommx-tests pass; lint + typecheck clean - [x] \`cargo insta test --accept\` for the 5 \`instance::logical_memory\` snapshots that gained the new \`Instance.named_function_metadata;…\` rows - [x] Format / clippy clean on changed files --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
termoshtt
added a commit
that referenced
this pull request
May 7, 2026
The release-note entry for #845 documents that to_bytes / from_bytes are removed from the non-top-level types in 3.0.0a3 (Function / Linear / Quadratic / Polynomial, Parameter, the NamedFunction family, the DecisionVariable family), but the migration guide only covered the Constraint-family removal that shipped in a2 alongside the Constraint.id rework. Add a dedicated §12 covering the broader removal, with the affected type list, rationale, and a Before/After example showing the container-type round-trip path. Cross-link from §2's existing Constraint-family aside, renumber the existing §12 (Convenience additions) to §13, and add a matching item to the migration checklist. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
termoshtt
added a commit
that referenced
this pull request
May 7, 2026
## Summary
Polish the v3.0 release notes and the Python SDK migration guide ahead
of cutting the `python-3.0.0a3` tag. No functional / source changes.
### Release notes (`docs/{en,ja}/release_note/ommx-3.0.md`)
- Promote the `## Unreleased` section to `## 3.0.0 Alpha 3` with the
GitHub Release badge matching the a1 / a2 entries.
- Add a one-line note under `## Unreleased` describing how new entries
are appended there as they land and promoted on each release.
- Fix four `v3.0.0a4` / `3.0.0a4` references that should have been
`3.0.0a3` (introduced in #847; EN+JA, lines 142 and 148 each).
- Add a 🆕 entry for the **AttachedX write-through wrappers** (#849,
#850, #852) — `AttachedConstraint` /
`AttachedIndicator/OneHot/Sos1Constraint` / `AttachedDecisionVariable`,
the `.detach()` escape hatch, and the `instance.decision_variables`
shape change to `list[AttachedDecisionVariable]`.
- Move the `removed_reason` column gate entry from the Alpha 2 section
into Alpha 3 (next to the related `*_df` accessor entry), and rephrase
to acknowledge that the initial `include=` gate already shipped in a2
(#796) while a3 finalizes the `kind=` / `include=` / `removed=` dispatch
shape (#847).
### Migration guide (`PYTHON_SDK_MIGRATION_GUIDE.md`)
- Add a new **§12** covering the 3.0.0a3 removal of `to_bytes` /
`from_bytes` from the broader non-top-level types — `Function` /
`Linear` / `Quadratic` / `Polynomial`, `Parameter`, the `NamedFunction`
family, the `DecisionVariable` family (#845). Includes a Before/After
example showing the container-type round-trip path.
- Cross-link from the existing §2 Constraint-family aside so readers
landing in either spot find the full picture, renumber the existing §12
(Convenience additions) to §13, and add a matching item to the migration
checklist.
After this lands, the tagging step in `DEVELOPMENT.md` (`git tag
python-3.0.0a3 && git push origin python-3.0.0a3`) will publish the
release through `release_python.yml`. The `pyproject.toml` files under
`python/ommx`, `python/ommx-tests`, and `python/ommx-*-adapter` are
already at `3.0.0a3` (set in a5d98a7).
## Test plan
- [ ] Visual review of `docs/en/release_note/ommx-3.0.md` and
`docs/ja/release_note/ommx-3.0.md` rendered output (Sphinx)
- [ ] Visual review of `PYTHON_SDK_MIGRATION_GUIDE.md` (GitHub markdown
— read on the PR diff)
- [ ] Confirm `pyproject.toml` versions across `python/ommx*` are
`3.0.0a3` so the tag picks up the right version
- [ ] Sanity-check that the Alpha 3 section's PR list matches what was
merged between `python-3.0.0a2` and the tag commit
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Drops bytes serialization (
to_bytes/from_bytes) from component-level Python wrappers. Top-level container types and cross-evaluate DTOs keep bytes serialization unchanged.This is a Python SDK breaking change, split out from #843 so the larger metadata-storage refactor can land separately. The Rust SDK is unchanged in this PR — only the PyO3 binding surface narrows.
Affected types
Removed
to_bytes/from_bytes:Function,Linear,Quadratic,PolynomialParameterNamedFunction,EvaluatedNamedFunction,SampledNamedFunctionDecisionVariable,EvaluatedDecisionVariable,SampledDecisionVariableKept (top-level + cross-evaluate DTOs):
Instance,ParametricInstance,Solution,SampleSetState,Samples,ParametersRationale
These methods originally existed because the pre-v3 Python SDK had its own protobuf-based wrapper layer, so every value crossing the Python ↔ Rust boundary had to be serialized —
to_bytes/from_byteswas the bridge. With v3's switch to direct PyO3 re-exports the boundary is gone, and element-level bytes round-trips no longer serve a purpose. The upcoming metadata-storage redesign (#843 lineage) would only inflate the maintenance cost of keeping them, so they're retired now.Test plan
task python:stubgenregenerated stubs anddocs/api/api_reference.jsontask python:testpasses (ommx core + highs / pyscipopt / python-mip / openjij adapters)task formatclean🤖 Generated with Claude Code